feat(theme): Add medium and high contrast theme - #5100
Conversation
…into hristototov/medium-and-high-contrast-theme
| /** Only `standard` uses the reference palette steps. The other levels need | ||
| * tones that are not in the palette, so they live in `./contrastSchemes`. */ |
There was a problem hiding this comment.
These don't match MD3's published contrast schemes. On the color roles page, light/high primary is md.ref.palette.primary20 (#381E72), but this ships #312259; secondary, outline, outline-variant and on-surface-variant are off too, and surface-container isn't meant to change with contrast at all. Every tone MD3 asks for is already a key of ref/palette.ts - could roleToTone just gain medium and high maps, the way the old // extend with 'medium' | 'high' when those ship comment planned?
There was a problem hiding this comment.
updated - I've removed the generator and hand-written the medium and high maps as you suggested.
| return { | ||
| ...base, | ||
| ...props.theme, | ||
| contrast: level, |
There was a problem hiding this comment.
Passing a theme that has its own colors silently overrides the contrast scheme, but theme.contrast still reports the level you asked for - so useTheme().contrast says "high" while the colours on screen are standard. Worth reconciling, given the docs tell people to read the level back off the theme.
There was a problem hiding this comment.
Good catch. The behaviour is intentional but was undocumented, which is the actual problem: contrast records which scheme was selected, and anything passed via theme.colors is layered on top, so an override survives a contrast change.
I've added code comments making that explicit.
| ? DarkTheme | ||
| : LightTheme; | ||
| ? getDynamicTheme(isDarkMode, contrast) | ||
| : createTheme({ dark: isDarkMode, contrast }); |
There was a problem hiding this comment.
This builds a new theme object on every render, so the provider rebuilds the theme and every themed component re-renders with it. getTheme(isDarkMode, contrast) returns the cached one worth using that here?
|
And we've got conflicts |
…into hristototov/medium-and-high-contrast-theme # Conflicts: # src/core/PaperProvider.tsx # src/index.tsx
| primaryFixedDim: 'primary80', | ||
| onPrimaryFixed: 'primary10', | ||
| onPrimaryFixedVariant: 'primary30', |
There was a problem hiding this comment.
I get a contrast ratio of 5.47:1 for onPrimaryFixedVariant (#4F378B) on primaryFixedDim (#D0BCFF), below the documented 7:1 target for high contrast. The dark/high mapping at lines 311-313 uses the same values. Could we adjust this pair and include it in the contrast assertions? CONTRAST_PAIRS currently checks onPrimaryFixed on primaryFixed, so it misses this combination.
| secondaryFixedDim: 'secondary80', | ||
| onSecondaryFixed: 'secondary10', | ||
| onSecondaryFixedVariant: 'secondary30', |
There was a problem hiding this comment.
I get 5.45:1 for onSecondaryFixedVariant (#4A4458) on secondaryFixedDim (#CCC2DC), below the documented 7:1 high-contrast target. The dark/high mapping at lines 315-317 has the same issue. Could we adjust this pair and cover it in the contrast assertions as well?
| tertiaryFixedDim: 'tertiary80', | ||
| onTertiaryFixed: 'tertiary10', | ||
| onTertiaryFixedVariant: 'tertiary30', |
There was a problem hiding this comment.
I get 5.49:1 for onTertiaryFixedVariant (#633B48) on tertiaryFixedDim (#EFB8C8), below the documented 7:1 high-contrast target. The dark/high mapping at lines 319-321 has the same issue. Could we adjust this pair and cover it in the contrast assertions as well? The test that requires all fixed colors to stay unchanged across contrast levels would also need to be revised.
satya164
left a comment
There was a problem hiding this comment.
Any additional themes should just be exported theme objects, e.g.:
MediumContrastLightThemeMediumContrastDarkThemeHighContrastLightThemeHighContrastDarkTheme
Similarly there can be variants for dynamic themes.
It's a much simpler concept since we already export themes and accept theme prop, and there's no need for additional helpers and utilities. "Building" a theme object with createTheme({ dark: true, contrast: 'high' }) instead of just importing HighContrastDarkTheme is unnecessary indirection.
Motivation
The theme only generates the MD3
standardcontrast level. MD3 defines three levels, and the higher two exist for accessibility: they raise the contrast between text and background so the UI stays readable for users with low vision and in bright light. Without them, apps built with Paper have no supported way to meet the raised contrast ratios.This adds the
mediumandhighschemes for light and dark, and acontrastprop onPaperProviderto select them.standardstays the default and its colours do not change. It usesscripts/generate-contrast-tokens.tsMeasured against the baseline
#6750A4seed:primarytoneonPrimaryContainertoneAPI
Related issue
Closes #5092
Test plan
Automated:
src/theme/__tests__/contrast.test.tschecks 17 text/background role pairs across both modes and both new levels, 64 pairs in total, and asserts >= 4.5 for medium and >= 7.0 for high. It also asserts thatLightThemeandDarkThemecolours are unchanged.src/core/__tests__/PaperProvider.test.tsxcovers thecontrastprop, thestandarddefault, prop precedence overtheme.contrast, and that a provider with onlycontrastset still follows the system color scheme.contrastfield on the serialized theme.Manual, on an Android 12 emulator (API 31), using the new Contrast control in the example app drawer under Preferences:
android-contrast.mov
ios-contrast.mov